-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Added JSON body processor #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Ulisses, Makefile.in ?
any reason for this ? |
Breno Makefile.in was added again for no apparent reason, sorry about that -- can you ignore it or do I need to submit a new pull request? Regarding modsecurity-recommended.conf -- no reason, since I used an existing modsecurity.conf in the tests I probably missed that noise in the end of the lines. Will fix right away. |
Ok. I test it tomorrow and commit. I will let you know any feedback. Thanks |
Ulisses, Getting some problems to compile modsecurity with yajl lib: apache2: Syntax error on line 208 of /etc/apache2/apache2.conf: Syntax error on line 8 of /etc/apache2/httpd.conf: Cannot load /usr/lib/apache2/modules/mod_security2.so into server: /usr/lib/apache2/modules/mod_security2.so: undefined symbol: yajl_parse I probably means that we are missing -lyajl So.. can you add a new build/find_yajl.m4 file to set the right @YAJL_CFLAGS@, @YAJL_LDADD@ variables into apache2/Makefile.am ? in find_yajl.m4 you should define a CHECK_YAJL function and call it from configure.ac. You can use the find_lua.m4 as an example. |
Breno, I added a check for yajl_parse in
I considered creating a m4 file anyway, just for consistency, but yajl does not provide a BTW, just as a sanity check -- do you have yajl installed? I used a package (http://packages.debian.org/stable/libdevel/libyajl-dev), which is for the 1.x version, instead of the current 2.x version (which apparently no Linux distribution is offering yet). |
Ulisses, I installed it, both 1.x and 2.x. Yes it is not *-config based package. Lua is not too. So find_lua.m4 file is a good example how to do this. I will try -config routines (because in some distros we have it) but also will try to find lua information without it. It is working in you linux because probably it is installed in some default paths. But this is not a real case. Many users install libs in optional directories. We need to handle it. Also... yajl 1.x is legacy and it is not being updated for at least 3 years :/ and play with |
.. and play with |
Acknowledged. I will add support for both yajl versions (1.x and 2.x) and use the |
Cool. Also could you please conditionally compile it ? like we do with lua. We have WITH_LUA macro do compile it. |
Sure. I knew I would very likely face more issues with the build system than with the code itself, so thanks for the tips. |
Yes. The code looks good! I'm starting some tests. Looks like we just need to improve the build system. when you define your find_yajl.m4 we will have a new --with-yajl option. So the users can set it to "no" and you just will not pass -DWITH_YAJL to the compiler. |
@urma Do you mind if i continue this? Besides this build system improvement, there is something else that should be done that you guys already know? @brenosilva? |
@urma's patches on the top of the current 'trunk' is available at: https://github.com/zimmerle/ModSecurity/tree/urma_json_parser Note however that it is not compiling due to some changes on the 'yajl' api. Not investigated yet. On my way to get it working. |
@zimmerle yajl 1.x is not compatible with 2.x -- my code was originally written for 1.x because that was what Debian offered as packages. I think I never got to adding the macros to wrap around the API differences (which are minor). It should also be taken care of in the autoconf m4 files. |
Just to keep it updated, the branch 'urma_json_parser' is working like a charm, and the branch 'urma_json_parser_with_json_collection_draft' works in the same fashion of the 'urma_json_parse' but adds the JSON inside a collection labeled JSON. They can be fetched from: |
merged into trunk branch. target for the next release. Thanks @urma ;) |
@zimmerle Any idea when this will be released? |
Hi @eoftedal, the target is our next release which should happens in the upcoming weeks. This is currently under the branch: Before merge it we have to fix a bug reported by a users which was testing it: |
Added support for a JSON body processor. Brief description:
{ 'data': { 'a': 1, 'b': 2 } } would result in ARGS:data.a = 1 and ARGS:data.b = 2, but ARGS:data would be undefined
[ { 'data': 1 }, { 'data': 2 } ] would result in ARGS:data = 1 and ARGS:data = 2, while
{ 'data': [ 1, 2 ] } would also result in ARGS:data = 1 and ARGS:data = 2
NOTE: Once again, Makefile.in got mixed in the commit for no apparent reason; please disregard the changes in it.